Conversation
WalkthroughAdds a reusable Android CI workflow and updates several existing GitHub Actions workflows: renames and consolidates static checks, replaces formatting check with Spotless, inlines Detekt, adds actionlint validation, and applies safer shell quoting and minor shell fixes across PR-quality and SDK size workflows. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (14)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gpunto
force-pushed
the
and-781/unify-android-ci
branch
3 times, most recently
from
c7e4766 to
5d517b3
Compare
| - name: Build and validate | ||
| run: ./gradlew build :plugin:validatePlugins | ||
|
|
||
| validate-workflows: |
There was a problem hiding this comment.
I took the chance to add a validation step for actions and workflows.
gpunto
force-pushed
the
and-781/unify-android-ci
branch
from
5d517b3 to
6c536d3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/android-ci.yml (1)
36-36: Consider using a stable ref instead of@mainfor the setup-gradle action.Referencing
@mainmeans any push to main could unexpectedly change behavior in consuming workflows. For stability, consider using a tagged version or commit SHA.🔎 Example:
- - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@main + - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@v1.0.0Also applies to: 45-45, 60-60
.github/workflows/ci.yml (1)
47-52: Add explicit step names for consistency.The checkout step in the
validate-workflowsjob lacks an explicitnameattribute, unlike other checkout steps in this workflow. For clarity, addname: Checkoutand also consider addingname: Validate workflowsto the actionlint step.Additionally, pinning to a commit SHA is the most secure option, though this is optional for well-maintained third-party actions.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/android-ci.yml(1 hunks).github/workflows/ci.yml(3 hunks)
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
13-30: LGTM - Well-structured static checks job.The consolidation of Spotless and Detekt into a single
static-checksjob is clean. Usingif: always()on Detekt ensures both checks run and report independently, which is good for comprehensive feedback on PRs..github/workflows/android-ci.yml (2)
69-76: Good fork-aware Sonar handling.The conditional logic to skip Sonar for fork PRs is a solid security practice since secrets are not available to workflows triggered from forks. The informational message on line 75-76 helps maintainers understand why Sonar was skipped.
63-68: Test results upload looks good.The
if: failure()condition ensures test results are only uploaded when tests fail, which is efficient. The glob pattern**will capture all test reports.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/android-ci.yml (2)
12-22: SONAR_TOKEN should be optional to avoid fork PR failures.This issue was previously flagged: marking
SONAR_TOKENasrequired: truewill cause workflow invocation failures for fork PRs that cannot supply the secret, even though the Sonar step is conditionally skipped for forks. Make itrequired: falseand update the Sonar condition (lines 70-73) to check both the repo match and secret availability.
51-53: Boolean input compared as string.This issue was previously flagged:
inputs.api-checkis defined astype: booleanbut the condition compares it to the string'true'. Use direct boolean evaluation instead.Apply this fix:
- if: always() && inputs.api-check == 'true' + if: always() && inputs.api-check
🧹 Nitpick comments (1)
.github/workflows/android-ci.yml (1)
36-36: Pin setup-gradle action to a specific version.The setup-gradle action references
@main, which can change unexpectedly and break CI. Pin to a specific tag or commit SHA for reproducibility and stability.Example fix:
- - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@main + - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@v1.0.0Apply this pattern to all three occurrences (lines 36, 45, 60).
Also applies to: 45-45, 60-60
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/android-ci.yml(1 hunks)
🔇 Additional comments (2)
.github/workflows/android-ci.yml (2)
62-62: Verify testCoverage task exists in consuming projects.The workflow invokes
./gradlew testCoverage, which is a custom task that may not exist in all Android projects. Ensure that any project calling this reusable workflow defines this task, or consider using standard tasks liketestwith coverage configuration.Would you like me to generate a script to check if testCoverage is defined in standard Android Gradle plugins, or would documentation suffice?
78-83: LGTM - validate-workflows job follows best practices.The actionlint validation is properly configured with a pinned version, providing good workflow quality checks.
gpunto
force-pushed
the
and-781/unify-android-ci
branch
from
6c536d3 to
34d9817
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/android-ci.yml (1)
51-53: Boolean input compared as string - use direct boolean evaluation.The
inputs.api-checkis defined astype: boolean, but the condition compares it to the string'true'. Boolean inputs should be compared directly without quotes.🔎 Apply this fix:
- name: API check - if: always() && inputs.api-check == 'true' + if: always() && inputs.api-check run: ./gradlew apiCheck
🧹 Nitpick comments (2)
.github/workflows/android-ci.yml (2)
36-36: Consider pinning the setup-gradle action to a specific version.The workflow references
@main, which can introduce unexpected breaking changes. For stability and reproducibility, pin to a specific tag or commit SHA.🔎 Example fix (apply to lines 36, 45, and 60):
- - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@main + - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@v1.0.0
69-73: Add check for SONAR_TOKEN availability in Sonar step condition.Since
SONAR_TOKENis now optional (required: false), the Sonar step should verify the token is available before attempting to use it, preventing potential failures when the token is not provided.🔎 Apply this fix:
- name: Sonar - if: github.event.pull_request.head.repo.full_name == github.repository + if: github.event.pull_request.head.repo.full_name == github.repository && secrets.SONAR_TOKEN != '' run: ./gradlew sonar env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/android-ci.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and validate
gpunto
force-pushed
the
and-781/unify-android-ci
branch
2 times, most recently
from
b3f961c to
02de4b3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/android-ci.yml (1)
71-75: Add SONAR_TOKEN availability check to prevent runtime failure.The Sonar step checks if the PR is from a fork, but doesn't verify that
SONAR_TOKENis actually available. Since the secret is now optional (required: falseat line 22), a caller might not provide it. If the token is missing when this step runs,./gradlew sonarwill likely fail.🔎 Recommended fix
Update the condition to also check token availability:
- name: Sonar - if: github.event.pull_request.head.repo.full_name == github.repository + if: github.event.pull_request.head.repo.full_name == github.repository && secrets.SONAR_TOKEN != '' run: ./gradlew sonar env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}Also update the skip message condition to be more accurate:
- name: Sonar skipped if PR from fork - if: github.event.pull_request.head.repo.full_name != github.repository + if: github.event.pull_request.head.repo.full_name != github.repository || secrets.SONAR_TOKEN == '' - run: echo "⚠️ Sonar skipped because the PR comes from a fork." + run: echo "⚠️ Sonar skipped because the PR comes from a fork or SONAR_TOKEN is not available."
🧹 Nitpick comments (1)
.github/workflows/android-ci.yml (1)
36-36: Pin setup-gradle action to a stable version.The
setup-gradleaction is referenced using@main, which can introduce instability if the main branch changes. This pattern appears in all jobs (lines 36, 45, 62).Best practice is to pin to a specific version tag or commit SHA for reproducibility and stability.
🔎 Example fix for all occurrences
Replace
@mainwith a specific version tag, e.g.,@v1.0.0(adjust based on available versions):- - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@main + - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@v1.0.0Apply this change to lines 36, 45, and 62.
Alternatively, use a commit SHA for even more precise pinning:
- - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@main + - uses: GetStream/stream-build-conventions-android/.github/actions/setup-gradle@abc1234
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/android-ci.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run static checks
- GitHub Check: Build and validate
🔇 Additional comments (5)
.github/workflows/android-ci.yml (5)
1-22: LGTM - Past review feedback addressed.The workflow inputs and secrets are properly configured. The
SONAR_TOKENis now marked as optional (required: false), which resolves the previous concern about fork PRs failing when they cannot provide the secret.
24-28: LGTM - Appropriate environment variable scope.The build cache secrets are correctly mapped at the workflow level for reuse across jobs, while
SONAR_TOKENis appropriately scoped to only the step that requires it.
40-53: LGTM - Boolean input correctly evaluated.The static checks are well-structured. The
always()conditions ensure that lint and API checks run even if earlier steps fail, and the boolean input comparison at line 52 correctly uses direct evaluation rather than string comparison (addressing previous feedback).
65-70: LGTM - Test results properly captured on failure.Uploading test results as artifacts on failure is a good practice for debugging. The path pattern correctly captures all test reports across modules.
84-85: No action required. v2.1.0 is the latest stable version of raven-actions/actionlint.
gpunto
force-pushed
the
and-781/unify-android-ci
branch
from
02de4b3 to
5b1d536
Compare
2 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.